[DirectX] Move triple/DL compat to bitcode writer #163587
Merged
+9
−14
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move our handling of setting up a compatible triple for DXIL to the bitcode writer itself, and do the same for the datalayout. This avoids us needing to temporarily change the triple just to change it back, and it also allows us to make the datalayout truly compatible, including the `i8:32` alignment that isn't accepted by modern LLVM's DataLayout.
|
@llvm/pr-subscribers-backend-directx Author: Justin Bogner (bogner) ChangesMove our handling of setting up a compatible triple for DXIL to the bitcode writer itself, and do the same for the datalayout. This avoids us needing to temporarily change the triple just to change it back, and it also allows us to make the datalayout truly compatible, including the Full diff: https://github.com/llvm/llvm-project/pull/163587.diff 2 Files Affected:
diff --git a/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp b/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp
index 82c43ff8dc352..26a8728e1f37c 100644
--- a/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp
+++ b/llvm/lib/Target/DirectX/DXILWriter/DXILBitcodeWriter.cpp
@@ -1165,12 +1165,15 @@ void DXILBitcodeWriter::writeValueSymbolTableForwardDecl() {}
/// Returns the bit offset to backpatch with the location of the real VST.
void DXILBitcodeWriter::writeModuleInfo() {
// Emit various pieces of data attached to a module.
- if (!M.getTargetTriple().empty())
- writeStringRecord(Stream, bitc::MODULE_CODE_TRIPLE,
- M.getTargetTriple().str(), 0 /*TODO*/);
- const std::string &DL = M.getDataLayoutStr();
- if (!DL.empty())
- writeStringRecord(Stream, bitc::MODULE_CODE_DATALAYOUT, DL, 0 /*TODO*/);
+
+ // We need to hardcode a triple and datalayout that's compatible with the
+ // historical DXIL triple and datalayout from DXC.
+ StringRef Triple = "dxil-ms-dx";
+ StringRef DL = "e-m:e-p:32:32-i1:8-i8:8-i16:32-i32:32-i64:64-"
+ "f16:32-f32:32-f64:64-n8:16:32:64";
+ writeStringRecord(Stream, bitc::MODULE_CODE_TRIPLE, Triple, 0 /*TODO*/);
+ writeStringRecord(Stream, bitc::MODULE_CODE_DATALAYOUT, DL, 0 /*TODO*/);
+
if (!M.getModuleInlineAsm().empty())
writeStringRecord(Stream, bitc::MODULE_CODE_ASM, M.getModuleInlineAsm(),
0 /*TODO*/);
diff --git a/llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp b/llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp
index 1eb03bfc087e3..725f2b1e7c76c 100644
--- a/llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp
+++ b/llvm/lib/Target/DirectX/DXILWriter/DXILWriterPass.cpp
@@ -149,11 +149,6 @@ class EmbedDXILPass : public llvm::ModulePass {
std::string Data;
llvm::raw_string_ostream OS(Data);
- Triple OriginalTriple = M.getTargetTriple();
- // Set to DXIL triple when write to bitcode.
- // Only the output bitcode need to be DXIL triple.
- M.setTargetTriple(Triple("dxil-ms-dx"));
-
// Perform late legalization of lifetime intrinsics that would otherwise
// fail the Module Verifier if performed in an earlier pass
legalizeLifetimeIntrinsics(M);
@@ -165,9 +160,6 @@ class EmbedDXILPass : public llvm::ModulePass {
// not-so-legal legalizations
removeLifetimeIntrinsics(M);
- // Recover triple.
- M.setTargetTriple(OriginalTriple);
-
Constant *ModuleConstant =
ConstantDataArray::get(M.getContext(), arrayRefFromStringRef(Data));
auto *GV = new llvm::GlobalVariable(M, ModuleConstant->getType(), true,
|
bogner
commented
Oct 15, 2025
|
|
||
| // We need to hardcode a triple and datalayout that's compatible with the | ||
| // historical DXIL triple and datalayout from DXC. | ||
| StringRef Triple = "dxil-ms-dx"; |
There was a problem hiding this comment.
It may make sense to assert here that M.getTargetTriple() is DXIL before ignoring it. This is all deep in the DirectX backend though so the value of that check seems kind of low.
llvm-beanz
approved these changes
Oct 15, 2025
inbelic
reviewed
Oct 15, 2025
inbelic
approved these changes
Oct 15, 2025
|
99 DML shaders fail to validate after this PR was merged: before and after I compiled one of the DML shaders with and without this PR and the difference in the dxil assembly is just the target datalayout Compiling the same DML shader with DXC, DXC gives the shader a datalayout of which matches the data layout that Clang emitted before this PR. Minimal reproducible test case // compile args: -T cs_6_7 -E CSMain -enable-16bit-types -Fo output.dat
groupshared float16_t smem[10240];
[numthreads(1, 1, 1)]
void CSMain() {
smem[0] = 0;
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Move our handling of setting up a compatible triple for DXIL to the bitcode writer itself, and do the same for the datalayout. This avoids us needing to temporarily change the triple just to change it back, and it also allows us to make the datalayout truly compatible, including the
i8:32alignment that isn't accepted by modern LLVM's DataLayout.